Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for selecting the Firestore emulator edition (standard or enterprise) through a new --edition CLI flag and firebase.json configuration. Key improvements suggested include hardening the edition selection logic to prevent crashes when the flag is used without a value, implementing stricter validation using enums in the JSON schema, and enhancing type safety by replacing generic strings with union types for the edition property.
I am having trouble creating individual review comments. Click here to see my feedback.
src/emulator/controller.ts (645-653)
The current logic for determining the emulator edition has a few issues:
- Potential Crash: If the
--editionflag is provided without a value (e.g.,firebase emulators:start --edition),options.editionwill betrueat runtime, causingcliEdition.toLowerCase()to throw an error. - Lack of Validation: Any string provided via CLI or config is accepted and logged as the edition, even if it's invalid.
- Complexity: The logic can be simplified to reduce nesting, as per the repository style guide.
Using utils.assertIsString and adding explicit validation against allowed values is recommended.
if (options.edition !== undefined) {
utils.assertIsString(options.edition, "edition");
}
const edition = (
(options.edition as string) ||
options.config.src.emulators?.firestore?.edition ||
"standard"
).toLowerCase();
if (edition !== "standard" && edition !== "enterprise") {
throw new FirebaseError(
"The Firestore emulator edition must be either 'standard' or 'enterprise'.",
{ exit: 1 },
);
}References
- Reduce nesting as much as possible. Handle edge cases early and exit or fold them into the general case. (link)
schema/firebase-config.json (1424-1426)
It is recommended to define an enum for the edition property in the JSON schema. This provides better validation and autocompletion for users editing their firebase.json file.
"edition": {
"type": "string",
"enum": ["standard", "enterprise"]
}src/firebaseConfig.ts (243)
Using a union type instead of a generic string for the edition property improves type safety and adheres to the repository's best practice of avoiding loose types when possible.
edition?: "standard" | "enterprise";
References
- Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. (link)
src/options.ts (29)
For better type safety, consider using a union type for the edition option, matching the allowed values for the Firestore emulator.
edition?: "standard" | "enterprise";
References
- Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. (link)
2 participants
Description
Add a new
--editionflag to the Firestore Emulator. Valid values are "standard" or "enterprise". The default value is "standard" if one is not provided. This changes also allows user to specifyeditionvia firebase.json. If command line flag is also present, it takes precedence over firebase.json.Scenarios Tested
Manually tested starting and querying various combinations of this flag.
Example:
Sample Commands
firebase emulators:start --only firestore --edition enterprise